Skip to content

Conversation

marmeladema
Copy link
Contributor

@marmeladema marmeladema commented Aug 31, 2020

Part of #70334

I followed the approach described by @eddyb and introduced a DefPathDataName enum.
To preserve compatibility, in various places, I had to rely on formatting manually by calling format!("{{{{{}}}}}", namespace).

My questions are:

  • Do we want to convert for places to use the new naming scheme? Or shall I re-add DefPathData::as_symbol but renamed as DefPathData::as_legacy_symbol to avoid manually allocating the legacy symbols?
  • Do we want to impl Display for DisambiguatedDefPathData to avoid manually calling write!(s, "{{{}#{}}}", namespace, component.disambiguator)?
  • We might also want to improve naming for DefPathDataName and DefPathData::get_name

r? @eddyb

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 31, 2020
@marmeladema marmeladema force-pushed the fix-closure-path-printing branch 2 times, most recently from 277e65a to 93ce1e4 Compare August 31, 2020 23:43
@marmeladema marmeladema force-pushed the fix-closure-path-printing branch 3 times, most recently from 7588122 to ee81a4e Compare September 1, 2020 18:35
@marmeladema marmeladema requested a review from eddyb September 1, 2020 18:43
@bors

This comment has been minimized.

@marmeladema marmeladema force-pushed the fix-closure-path-printing branch from 44c6c76 to aeace7f Compare September 8, 2020 22:33
@eddyb
Copy link
Member

eddyb commented Sep 25, 2020

@bors r+ Thanks!

@bors
Copy link
Collaborator

bors commented Sep 25, 2020

📌 Commit 6785568870cac091755900f1623795535f41591b has been approved by eddyb

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 25, 2020
@marmeladema marmeladema force-pushed the fix-closure-path-printing branch from 6785568 to 5946c12 Compare September 25, 2020 21:54
@eddyb
Copy link
Member

eddyb commented Sep 25, 2020

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 25, 2020

📌 Commit 5946c12 has been approved by eddyb

@bors
Copy link
Collaborator

bors commented Sep 26, 2020

⌛ Testing commit 5946c12 with merge c6622d1...

@bors
Copy link
Collaborator

bors commented Sep 26, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: eddyb
Pushing c6622d1 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants